fix(slf): restore vanilla shadow lights when disabled#101
Conversation
|
Warning Review limit reached
More reviews will be available in 58 minutes and 55 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Restores vanilla point/spot shadow-light behavior when Shadow Limit Fix (SCM) is disabled at boot by making ShadowCasterManager::GetShadowSlot() fall back to the engine’s own shadowmapDescriptors[0].shadowmapIndex instead of relying on the SCM pool (which is unpopulated when hooks aren’t installed). This prevents shadow-casting lights from being “handled then dropped” by downstream consumers that interpret -1 as “skip”.
Changes:
- Added a boot-latched
ShadowCasterManager::IsActive()API to express whether SCM hooks are actually live this session (enabled at boot + no external conflict). - Implemented an engine-descriptor-based fallback slot lookup (
GetEngineShadowSlot) used when SCM is inactive. - Updated
GetShadowSlot()to route through the fallback when SCM is inactive, preserving the existing-1sentinel for directional/sun lights.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/Features/LightLimitFix/ShadowCasterManager.h |
Documents and exposes IsActive() and clarifies GetShadowSlot() behavior when SCM is inactive. |
src/Features/LightLimitFix/ShadowCasterManager.cpp |
Implements IsActive(), adds vanilla descriptor fallback slot resolution, and gates SCM pool lookup accordingly. |
alandtse
changed the title
Disabling Shadow Limit Fix at boot already skips every SCM game hook, so the engine's shadow scheduler runs untouched. But the LightLimitFix cluster pipeline was not gated on SCM and still routed every shadow light through ShadowCasterManager::GetShadowSlot(). With the scheduler hook never installed the SCM pool stays empty, so GetShadowSlot() returned -1 for every light. UpdateLights() had already inserted each light into shadowLightPtrs (marking it "handled"), then dropped it on the -1 early-return, and the activeLights fallback skipped it too -- every shadow-casting light contributed zero to the cluster, blacking out all shadowcasters (spots and omnis). CopyShadowLightData() shared the same dependency, leaving the Shadows (t102) buffer empty. Add ShadowCasterManager::IsActive() (boot-latched: true only when SCM installed its hooks and no external conflict was detected) and make GetShadowSlot() fall back to the engine's own shadowmapDescriptors[0].shadowmapIndex when inactive -- the exact vanilla slice the engine rendered into, with the directional sun mapped to -1. All three consumers (cluster builder, strict-light setup, shadow-data copy) route through GetShadowSlot(), so point/spot casters now light and shadow exactly as vanilla LLF would without SLF. SCM-on behavior is unchanged. Downstream bounds checks already clamp any out-of-range slice safely. Fixes #97 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Trim the IsActive / GetEngineShadowSlot / GetShadowSlot doc and inline comments to concise "why" notes per the repo comment standard; no code change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
alandtse
force-pushed
the
claude/objective-clarke-2b2ede
branch
from
22391ed to
f1ad17e
Compare
|
No actionable suggestions for changed features. |
|
✅ A pre-release build is available for this PR: |
Problem
Fixes #97 — with Shadow Limit Fix unticked and the game restarted, all shadowcasters stop emitting light (both spots and omnis).
Disabling SLF at boot already skips every SCM game hook (
Installearly-returns), so the engine's shadow scheduler runs untouched. But the LightLimitFix cluster pipeline was not gated on SCM and still routed every shadow light throughShadowCasterManager::GetShadowSlot():s_lights) stays empty →GetShadowSlot()returns-1for every light.UpdateLights()inserts each shadow light intoshadowLightPtrs(marking it "handled"), then drops it on thestableSlot < 0early-return; theactiveLightsfallback loop skips it too. Net: shadow-casting lights contribute zero to the cluster → blacked out.CopyShadowLightData()shared the same dependency, leaving theShadows(t102) buffer empty.Fix
One central change — all three consumers (cluster builder, strict-light setup, shadow-data copy) route through
GetShadowSlot():ShadowCasterManager::IsActive()— boot-latched; true only when SCM installed its hooks at boot and no conflicting external plugin was detected.GetShadowSlot()now falls back to the engine's ownshadowmapDescriptors[0].shadowmapIndexwhen SCM is inactive — the exact vanilla slice the engine rendered into, with the directional sun mapped to-1. VR-aware (mirrorsSetShadowParameters).Result: with SLF off, point/spot casters light and shadow through the existing kSHADOWMAPS (t102/t103) path, exactly as vanilla LightLimitFix would — matching the issue's "no change from the game" intent. SCM-on behavior is unchanged. Downstream bounds checks already clamp any out-of-range slice safely.
Testing
ALLpreset clean (0 compile/link errors), deployed to SE + VR.🤖 Generated with Claude Code